Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb7d998705
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| candidate = current / STRATEGY_FILE | ||
| try: | ||
| if candidate.is_file(): | ||
| return candidate, repo_root |
There was a problem hiding this comment.
find_strategy_file currently returns the first STRATEGY.md encountered while walking upward, so running from a subdirectory that contains its own STRATEGY.md will select that local file instead of the repo-root strategy. This reintroduces nearest-ancestor behavior that the new strategy design explicitly tries to avoid, and downstream consumers (strategy status/read/list and skills that call them) will ground on different strategy docs depending on cwd. In a monorepo or any repo with an accidental nested STRATEGY.md, this causes inconsistent planning/review behavior and false conflict/drift signals.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in dad66e3 — replaced the upward-walk loop in find_strategy_file with a direct check at repo_root / STRATEGY.md. STRATEGY_WALK_MAX_DEPTH stays as a dead constant for backward compat with any importer. New regression guard T3b in strategy_smoke_test.sh creates a fixture with both repo-root and apps/web/ STRATEGY.md, runs status from the subdir, and asserts resolution picks the repo-root file (verifies via name field round-trip).
…e-effect chain The "When do I use prospect vs capture vs interview vs plan?" FAQ now covers all five upstream-of-work commands (added strategy as the most-upstream), shows (Strategy) as optional prefix on existing pathways, and explains that glossary terms + decision records accumulate as a side effect of /flow-next:interview --docs (no separate "build my glossary" route needed). Mirrors the README route-map updates from PR gmickel/flow-next#125.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcf0bbd893
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if text == STRATEGY_HUSK_SENTINEL: | ||
| return False |
There was a problem hiding this comment.
_strategy_section_filled only treats _Not currently tracking._ as empty, but /flow-next:strategy Phase 1.3 writes _Not yet captured._ into every unanswered required section during partial saves. In an interrupted first run, strategy status then reports those placeholder sections as populated, so sections_filled becomes non-zero (often 5) and downstream skills incorrectly enable strategy-aware grounding/conflict checks against placeholder text. This breaks the intended “resume first-run interview” and husk/signal gating behavior after mid-flow abandonment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in dad66e3 — added STRATEGY_DRAFT_PLACEHOLDER = "_Not yet captured._" and a unified STRATEGY_EMPTY_SENTINELS frozenset that _strategy_section_filled consults; future sentinels add to the set rather than growing scattered if/elif checks. New regression guard T6b in strategy_smoke_test.sh creates a partial-save fixture with 2 populated + 3 placeholder sections and asserts sections_filled == 2 (not 5). All smoke + ci green.
…entinel (PR #125 review) Two bugs surfaced by Codex review on PR #125: P2 — find_strategy_file walked upward and returned the first STRATEGY.md encountered, so a subdir with its own STRATEGY.md (intentional or accidental) hijacked resolution. This is exactly the cascade the design explicitly rejected (decision context: "single-root, NOT nearest-ancestor like glossary — strategy is repo-wide by Rumelt's definition"). R7's spec language was internally contradictory; worker implemented the literal first half, not the design intent. Fix: replace the walk with a direct check at repo_root / STRATEGY.md. STRATEGY_WALK_MAX_DEPTH stays as a dead constant for backward compat with any importer; it has no effect. P1 — _strategy_section_filled only treated _Not currently tracking._ as empty, but the strategy skill writes _Not yet captured._ into every unfilled required section during atomic per-section partial saves. After mid-flow abandonment, sections_filled would inflate to 5 instead of matching the answered-section count, falsely activating downstream strategy-aware grounding against placeholder text. Fix: introduce STRATEGY_DRAFT_PLACEHOLDER and a unified STRATEGY_EMPTY_SENTINELS frozenset that _strategy_section_filled consults; future sentinels add to the set. Smoke regression guards: - T3b: subdir with its own STRATEGY.md is ignored (single-root) - T6b: _Not yet captured._ placeholder excluded from sections_filled ci_test 57/57, strategy_smoke 62/62, glossary_smoke 80/80, sync-codex green.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dad66e3e7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| STRATEGY_AWARE=0 | ||
| else | ||
| STRAT_FILLED=$("$FLOWCTL" strategy status --json 2>/dev/null | jq -r '.sections_filled // 0') | ||
| if [[ "${STRAT_FILLED:-0}" -ge 1 ]]; then |
There was a problem hiding this comment.
The flag matrix in this skill says --no-docs should turn Glossary/Decisions/and Strategy off, but this block still auto-enables STRATEGY_AWARE from strategy status whenever sections_filled >= 1. In repos with a populated STRATEGY.md, /flow-next:interview --no-docs will still run behavior (e) and surface strategy-conflict questions/sections, which violates the documented override contract and user intent to disable doc-aware additions for that run.
Useful? React with 👍 / 👎.
- New /flow-next:strategy skill writes/maintains repo-root STRATEGY.md (peer of GLOSSARY.md / README.md). 5 required + 2 optional sections derived from Richard Rumelt's strategy kernel (diagnosis / guiding policy / coherent action) extended with persona + metrics for repo-doc utility - Single-root resolution at repo root only (NOT nearest-ancestor like glossary — strategy is repo-wide by Rumelt's definition) - flowctl strategy status / read / list — read-only plumbing; the skill IS the editor (no add/edit subcommands) - Doc-aware autodetect extended with third condition (strategy.sections_filled >= 1); 5-row flag matrix where --docs / --no-docs cascade to all three categories and explicit --strategy / --no-strategy always wins over the cascade - Downstream grounding in 5 skills: prospect (Phase 0 scan + out-of-scope-vs-strategy rejection), plan (## Strategy Alignment + drift block), interview (## Strategy Conflicts), capture ([strategy:<track>] source-tag + --override-strategy with decision-record prompt), plan-sync (drift surfacing, never auto-supersedes) - R19 strategy-doc fluff guard (Tier 1: synergy / pivot / disrupt / thought-leadership / best-in-class / world-class / 10x); two-tier (canonical ci_test.sh section 5d + sync-codex.sh validation) - Version bump 0.40.0; descriptions updated to 21 subagents / 17 commands / 22 skills; Codex mirror regenerated - Smoke test strategy_smoke_test.sh (T1-T12, 62 assertions, ~5s); CI matrix runs ci_test.sh on ubuntu/macos/windows - Docs: CHANGELOG, root README, plugins/flow-next/README, CLAUDE.md, .flow/usage.md
gmickel
force-pushed
the
fn-39-project-strategy-strategymd-anchor
branch
from
dad66e3 to
c4cb09f
Compare
1 participant
Summary
/flow-next:strategy— agent-native skill that writes/maintains a repo-rootSTRATEGY.md(problem, approach, personas, metrics, tracks). Mirror of CE 3.4'sce-strategywith flow-next adaptations (drop Marketing section, bareAskUserQuestionin canonical, lead-with-recommendation only on routing questions).out-of-scope-vs-strategyrejection), plan (## Strategy Alignment+ drift block), interview (autodetect +## Strategy Conflicts), capture ([strategy:<track>]source tag +--override-strategyflag with decision-record prompt), plan-sync (drift surfacing, never auto-supersedes).ci_test.sh+sync-codex.shvalidation) — separate from R17 DDD guard.What's in this PR
plugins/flow-next/scripts/flowctl.pycmd_strategy_status/read/list+ parse/render/walk helpers; single-root walk (NOT nearest-ancestor like glossary)plugins/flow-next/skills/flow-next-strategy/plugins/flow-next/commands/flow-next/strategy.mdplugins/flow-next/skills/flow-next-{prospect,plan,interview,capture,sync}/plugins/flow-next/agents/plan-sync.mdplugins/flow-next/scripts/ci_test.shscripts/sync-codex.shplugins/flow-next/scripts/strategy_smoke_test.shplugins/flow-next/codex/**Architectural calls baked in
.flow/wipe, R18 invariant from glossary epic.flowctl strategy add/edit/remove. flowctl is read-only plumbing (status/read/list).generator: flow-next-strategyfrontmatter sentinel; multi-format migration deferred to v2.AskUserQuestionin canonical — sync-codex rewrites torequest_user_input; no inline cross-platform tables polluting agent context.Test plan
strategy_smoke_test.sh— T1-T12 56/56 pass, 4.3s, refuses to run from main plugin repoci_test.sh— 57/57 pass (includes new R19 guard); fluff-fixture injection confirms guard fires non-zero on seededsynergysmoke_test.sh— 130/130 pass (regression check)glossary_smoke_test.sh— 80/80 pass (regression check)sync-codex.sh— zero errors; Codex mirror hasrequest_user_inputsubstituted forAskUserQuestion;flow-next-strategy/agents/openai.yamlgenerated